Custom timeout for delayed event restarts #4921
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
toger5
requested review from
florianduros and
MidhunSureshR
and removed request for
a team
4 tasks
toger5
force-pushed
the
toger5/custom-timeout-for-delayed-restart
branch
from
d056570 to
a4310d8
Compare
toger5
force-pushed
the
toger5/custom-timeout-for-delayed-restart
branch
from
a7abdf6 to
fae4ca4
Compare
toger5
changed the base branch from
fkwp/matrixrtc/delayed_event_reset_timeout
to
develop
toger5
force-pushed
the
toger5/custom-timeout-for-delayed-restart
branch
from
fae4ca4 to
6e5ca15
Compare
toger5
force-pushed
the
toger5/custom-timeout-for-delayed-restart
branch
from
6e5ca15 to
44edf0e
Compare
robintown
reviewed
Jul 18, 2025
There was a problem hiding this comment.
Where should I be looking to find the default of 2300 ms?
And hmm, I'm not quite finding this solution elegant. I do like Florian's solution to cover the Element Call SPA case. For the widget case, have we considered setting 2300 ms as an opinionated default for all widget API calls to this endpoint in Element Web?
It would be part of EC. The PR does not yet exist but would be a one line PR to add the config param. |
This has been done in the rust-sdk. https://github.com/matrix-org/matrix-rust-sdk/pull/5413/files |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(alternative to: #4896)
This PR introduces a new setting
delayedEventRestartLocalTimeoutMS.This setting is used in the context of MatrixRTC. We need to restart the dealyed events to make sure
the HomeServer is sending the delayed rtc leave event. In bad network environments we might end up
waiting for too long for the event to arrive and we will not send another restart event until the local timeout is reached.
In those scenarios chances for success are higher if we use a lower local timeout to increase the tries we do instead of waiting
for responses on requests which are stuck.
It also sets the default value for it to: 2300 which works well.
The reason why we make this a global configuration instead of sth we pass through via the
requestOptionsas it is done in: #4896 is because of how Element Call is implemented.In SPA mode we can control where we send
POST /_matrix/client/v1/delayed_events/{delay_id}. But in widget mode we cannot configure the exact call to the enpoint. The widget driver will just use the raw function.Delayed event restarts in general are a very cheap server operation. So a smaller timeout in every case seems appropriate.
Checklist
public/exportedsymbols have accurate TSDoc documentation.